get errors for a notebook URI #1247
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for retrieving diagnostics/errors from Jupyter notebook URIs by implementing special handling for notebook cells. When a notebook URI is detected, the tool now iterates through all cells in the notebook and collects diagnostics from each cell's document rather than trying to get diagnostics from the notebook URI directly.
Key changes:
- Added notebook URI detection and specialized diagnostic collection for notebook cells
- Reorganized imports and moved diagnostic filtering logic for better code structure
- Added error logging when notebook documents cannot be found
|
|
||
| private getNotebookCellDiagnostics(uri: URI) { | ||
| const notebook = this.workspaceService.notebookDocuments | ||
| .find((doc: { uri: URI }) => doc.uri.toString() === uri.toString()); |
There was a problem hiding this comment.
The URI comparison using toString() is inefficient and may not handle URI normalization correctly. Consider using URI's built-in comparison methods like uri.toString() === doc.uri.toString() with proper URI normalization, or use VS Code's URI comparison utilities if available.
| .find((doc: { uri: URI }) => doc.uri.toString() === uri.toString()); | |
| .find((doc: { uri: URI }) => URI.equals(doc.uri, uri)); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
that's not a real function
There was a problem hiding this comment.
I think the better approach is to use isEqual function found in resources.ts
|
|
||
| diagnostics = diagnostics.filter(d => d.severity <= DiagnosticSeverity.Warning); | ||
| let diagnostics: vscode.Diagnostic[] = []; | ||
| if (isJupyterNotebookUri(uri)) { |
There was a problem hiding this comment.
Shouldn't this be used for all notebooks, not just Jupyter?
I think you can use INotebookService.hasSupportedNotebook to check if Uri is a notebook document
|
|
||
| private getNotebookCellDiagnostics(uri: URI) { | ||
| const notebook = this.workspaceService.notebookDocuments | ||
| .find((doc: { uri: URI }) => doc.uri.toString() === uri.toString()); |
There was a problem hiding this comment.
I think the better approach is to use isEqual function found in resources.ts
| } | ||
|
|
||
| private getNotebookCellDiagnostics(uri: URI) { | ||
| const notebook = this.workspaceService.notebookDocuments |
There was a problem hiding this comment.
I would suggest using findNotebook function from notebooks.ts (that does what you're doing here and you also don't need to do the uri.toString)
fix microsoft/vscode#268430
the language service works with text document URIs, which cells have, but not Notebook URIs.
This change collects diagnostics for all cells within the requested notebook.